Conversation
…the Toomre model in the updated list of valid types
…ivative form, which is more accurate near the center
Contributor
There was a problem hiding this comment.
Pull request overview
Adds a Toomre disk option for cylindrical-basis deprojection/conditioning, expands the Abel inversion implementations used during deprojection, and introduces several standalone test/verification executables under utils/Test.
Changes:
- Add a new
Toomredisk model and wire a configurable deprojection model selector (dmodel) intoCylindricalbasis initialization. - Update
EmpCylSL::create_deprojectionto support multiple Abel inversion variants (Derivative/Subtraction/IBP) with derivative as default. - Add
Deprojector/EmpDeprojtest utilities and build targets inutils/Test.
Reviewed changes
Copilot reviewed 15 out of 15 changed files in this pull request and generated 16 comments.
Show a summary per file
| File | Description |
|---|---|
| utils/Test/testEmpDeproj.cc | New CLI test comparing EmpDeproj vs Deprojector across profiles/methods |
| utils/Test/testDeproject.cc | New example program for Deprojector (sampled + analytic construction) |
| utils/Test/testDeprojPlummer.cc | New simple deprojection verification driver (currently hardwired) |
| utils/Test/EmpDeproj.cc | New empirical deprojection implementation used by test programs |
| utils/Test/EmpDeproj.H | Header for EmpDeproj test-class API |
| utils/Test/Deprojector.cc | New numerical Abel deprojection implementation and grid integration logic |
| utils/Test/Deprojector.H | Header/API for Deprojector |
| utils/Test/CubicSpline.cc | New spline helper for sampled-data deprojection path |
| utils/Test/CubicSpline.H | Header for CubicSpline |
| utils/Test/CMakeLists.txt | Adds new test executables to the build |
| include/EmpCylSL.H | Adds AbelType + abel_type member to control deprojection variant |
| include/DiskModels.H | Adds Toomre disk density model for deprojection conditioning |
| exputil/EmpCylSL.cc | Switchable Abel inversion variants used in deprojection pipeline |
| expui/BiorthBasis.cc | Adds dmodel config key + lookup map and uses it when deproject is enabled |
| expui/BiorthBasis.H | Introduces DeprojType enum + lookup map for deprojection disk models |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
…Deproj for method variables
Contributor
There was a problem hiding this comment.
Pull request overview
Adds a Toomre-based option for cylindrical deprojection and refines the numerical Abel inversion machinery, along with several small test/verification executables under utils/Test.
Changes:
- Add a new Toomre disk model for deprojection and wire it into cylindrical basis initialization via a
dmodellookup. - Introduce multiple Abel inversion variants (derivative/subtraction/IBP) in the EmpCylSL deprojection path (defaulting to derivative).
- Add standalone
utils/Testdeprojection utilities (Deprojector,EmpDeproj) and test drivers, and hook them intoutils/Test/CMakeLists.txt.
Reviewed changes
Copilot reviewed 16 out of 16 changed files in this pull request and generated 12 comments.
Show a summary per file
| File | Description |
|---|---|
| utils/Test/testEmpDeproj.cc | New CLI test comparing EmpDeproj against Deprojector for several analytic profiles. |
| utils/Test/testEmp.cc | New CLI test for donut-shaped (inner cut) empirical deprojection. |
| utils/Test/testDeproject.cc | New minimal examples for Deprojector from sampled data vs functor. |
| utils/Test/testDeprojPlummer.cc | New Plummer/Gaussian/Toomre Deprojector validation driver. |
| utils/Test/EmpDeproj.cc | Adds an empirical deprojection implementation used by the new test drivers. |
| utils/Test/EmpDeproj.H | Declares the EmpDeproj helper class and AbelType selection. |
| utils/Test/Deprojector.cc | Adds a standalone deprojection implementation with tail-matching and improved inner integration handling. |
| utils/Test/Deprojector.H | Declares the standalone Deprojector API used by tests. |
| utils/Test/CubicSpline.cc | Adds a basic natural cubic spline for sampled-data deprojection. |
| utils/Test/CubicSpline.H | Declares the cubic spline used by Deprojector sampled-data ctor. |
| utils/Test/CMakeLists.txt | Registers the new utils/Test executables in the build. |
| include/EmpCylSL.H | Adds AbelType state to EmpCylSL for selecting deprojection variant. |
| include/DiskModels.H | Adds a new Toomre disk density model (used for deprojection conditioning). |
| exputil/EmpCylSL.cc | Updates EmpCylSL’s Abel inversion integral to support multiple variants. |
| expui/BiorthBasis.cc | Adds dmodel string parsing + lookup for selecting deprojection model (mn/exponential/toomre/python). |
| expui/BiorthBasis.H | Adds DeprojType and makes DiskType an enum class for stronger typing. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
Pull request overview
This PR enhances cylindrical-basis deprojection support by adding a Toomre disk option and improving the numerical Abel inversion implementation, along with new test/verification utilities under utils/Test.
Changes:
- Add a Toomre disk model for cylindrical deprojection to provide a less-cuspy alternative to exponential.
- Introduce selectable Abel inversion variants (derivative/subtraction/IBP) and change the default to the internal-derivative form.
- Add standalone test programs and supporting spline/deprojection utilities for verification.
Reviewed changes
Copilot reviewed 15 out of 15 changed files in this pull request and generated 12 comments.
Show a summary per file
| File | Description |
|---|---|
expui/BiorthBasis.cc |
Adds dmodel string handling and deprojection-model selection (mn/exponential/toomre/python) + pyproj config. |
expui/BiorthBasis.H |
Extends Cylindrical config/state with pyproj, adds DeprojType, and converts DiskType to enum class. |
exputil/EmpCylSL.cc |
Updates Abel inversion implementation and routes through a new abel_type selection. |
include/EmpCylSL.H |
Adds AbelType enum + abel_type member to control deprojection inversion method. |
include/DiskModels.H |
Adds a Toomre disk model usable for deprojection conditioning. |
utils/Test/CMakeLists.txt |
Builds new test executables (testDeproj, testEmpDeproj, testEmp) and links Python embed for testEmp. |
utils/Test/testDeproject.cc |
New CLI tool to validate Deprojector against analytic profiles. |
utils/Test/testEmpDeproj.cc |
New CLI tool comparing EmpDeproj vs Deprojector across profiles/Abel variants. |
utils/Test/testEmp.cc |
New CLI tool driving EmpDeproj, optionally via embedded Python-provided functions. |
utils/Test/EmpDeproj.cc |
Implements a test-local EmpDeproj (EmpCylSL-like) deprojection routine with selectable Abel variants. |
utils/Test/EmpDeproj.H |
Declares the test-local EmpDeproj class API. |
utils/Test/Deprojector.cc |
Implements an independent Abel deprojector with improved inner-radius systematics. |
utils/Test/Deprojector.H |
Declares the Deprojector API and data-driven/analytic constructors. |
utils/Test/CubicSpline.cc |
Adds a minimal natural cubic spline implementation used by Deprojector. |
utils/Test/CubicSpline.H |
Declares the minimal spline interface used by Deprojector. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Member
Author
|
@copilot You suggested changing |
Co-authored-by: The9Cat <25960766+The9Cat@users.noreply.github.com>
Fix clang build: constexpr pi initialization
Contributor
There was a problem hiding this comment.
Pull request overview
This PR enhances cylindrical basis deprojection support by adding a Toomre disk option and improving/expanding the numerical Abel inversion approach used during deprojection and verification, along with several new standalone test drivers under utils/Test.
Changes:
- Add a Toomre disk model for smoother, less-cuspy cylindrical deprojection.
- Introduce selectable Abel inversion variants (derivative/subtraction/IBP) in
EmpCylSLdeprojection logic (defaulting to derivative). - Add standalone C++ test/verification utilities for deprojection and Abel transform behavior, including optional Python-supplied surface-density functions.
Reviewed changes
Copilot reviewed 15 out of 15 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| utils/Test/testEmpDeproj.cc | New CLI test comparing EmpDeproj vs Deprojector for multiple analytic profiles. |
| utils/Test/testEmp.cc | New CLI test driver supporting Python-embedded callable for Σ(R) or Σ(R,z). |
| utils/Test/testDeproject.cc | New independent Deprojector test driver for analytic profiles. |
| utils/Test/EmpDeproj.cc | Adds an empirical deprojection helper implementation with multiple Abel variants (test utility). |
| utils/Test/EmpDeproj.H | Declares the EmpDeproj test utility API and AbelType enum. |
| utils/Test/Deprojector.cc | Adds an independent deprojection implementation and improved near-origin handling (test utility). |
| utils/Test/Deprojector.H | Declares Deprojector test utility API (analytic- and sampled-data constructors). |
| utils/Test/CubicSpline.cc | New natural cubic spline implementation used by Deprojector (test utility). |
| utils/Test/CubicSpline.H | Declares the spline implementation (test utility). |
| utils/Test/CMakeLists.txt | Wires new test executables (testDeproj, testEmpDeproj, testEmp) into the utils/Test build. |
| include/EmpCylSL.H | Adds internal AbelType and abel_type state for deprojection. |
| include/DiskModels.H | Adds Toomre disk model for deprojection inputs. |
| exputil/EmpCylSL.cc | Updates Abel inversion integral logic to support multiple numerical variants. |
| expui/BiorthBasis.cc | Adds dmodel mapping/selection for deprojection model types, plus pyproj support. |
| expui/BiorthBasis.H | Adds deprojection model typing (DeprojType), pyproj, and modernizes DiskType to enum class. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Movitation
Needed a less cuspy option than exponential for attempting to produce a doughnut-shaped basis. So this adds a Toomre model for that purpose.
Improvements
Cylindricalparameter parsing: thedmodelkey was read as aboolrather than astring.utils/Tests. These methods could be made user selectable but I have not done this in this PR.testEmpthat uses the same algorithm asEmpCylSLis included inutils/Test. I also provided two basic sanity checks on Abel transforms for verification and debugging. These could be removed.Notes and comments
Examples